Update Gradle Version Formatting Validation #10326
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Gradle version validation to use a more specific regular expression and provides a more detailed error message for invalid formats. The corresponding tests have been improved to cover more invalid version scenarios. While these changes are good, a new constant agpForExamples has been added but is not used anywhere in the command's logic, which should be addressed.
jesswrd
changed the title
| 'A version with a valid format (maximum 2-3 numbers separated by period) must be provided.'); | ||
| printError(''' | ||
| A version with a valid format (maximum 2-3 numbers separated by 1-2 periods) must be provided. | ||
| 1. The first number must have a single digit |
There was a problem hiding this comment.
I think the other conditions are reasonable, but I think given that we are currently on gradle 9 we may come to regret implementing this first condition. I think we should wait till the release after gradle 9 to see where they go with version numbering to avoid shooting ourselves in the foot here.
Also, I don't know the history of this code, but is there a reason we don't allow release candidates here? Fixing this wouldn't be blocking (as the old regex doesn't allow it either, so this is an improvement regardless), but for ex 9.1.0-rc-4 is indeed a valid gradle version, though neither regex here considers it so.
There was a problem hiding this comment.
I realize we are on Gradle 9 now and Gradle 10 would probably break that first condition. I can just make the change now to allow for 1-2 digits. I originally figured that the change could be made once Gradle 10 actually rolled around.
As for release candidates—I think the main use case of update_dependency_command is to give us on the Android team an easy way of bulk updating android dependencies in packages whenever the flutter/flutter templates get updated. I don't think we really use release candidates versioning for templates?
There was a problem hiding this comment.
I think the main use case of update_dependency_command is to give us on the Android team an easy way of bulk updating android dependencies in packages whenever the flutter/flutter templates get updated. I don't think we really use release candidates versioning for templates?
I was not familiar with this command/tool but that makes sense
2 participants
Updated the Gradle version formatting validation with a stricter regex. Previously passed on invalid versions.
Pre-Review Checklist
[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under1.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under1.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2 ↩3